Skip to content

Safety: require 100% branch coverage - #3736

Closed
joedlai wants to merge 3 commits into
commaai:masterfrom
joedlai:bounty/branch-coverage
Closed

Safety: require 100% branch coverage#3736
joedlai wants to merge 3 commits into
commaai:masterfrom
joedlai:bounty/branch-coverage

Conversation

@joedlai

@joedlai joedlai commented Sep 10, 2026

Copy link
Copy Markdown

Superseded by #3742 to submit this work from khchen428. The code and commits are unchanged. Please continue review on the replacement PR.

The safety suite has 157 uncovered branches with GCC, and the CI gate only requires line coverage. Add regressions for partial wheel/brake/pedal signals, cruise and button transitions, malformed actuation and diagnostic payloads, variant isolation, and common safety configuration failures. Remove dispatch checks already implied by the active RX/TX descriptors and simplify exhaustive callbacks, then require 100% line and branch coverage.

Safety behavior is intended to remain unchanged. Configuration-dependent guards that are reachable, including Nissan's alternate cruise bus and Hyundai powertrain selection, remain and gain tests. The existing test-harness exclusion and mutation survivor list are unchanged.

Ignoring indentation, the production changes are 57 added and 143 removed lines across 23 headers; most of the larger header diffs come from removing an outer dispatch check and unindenting its body.

Validation:

  • GCC and Apple Clang safety suites: 3,381 tests, 410 skipped; both 100% line and branch gates pass.
  • Independent paired libraries: 11,080,896 API calls across the complete safety suite agree with the unmodified baseline; the comparator also detects an injected divergence.
  • Independent review enumerated all 65,536 parameters for every registered mode in debug and release builds and checked each removed guard's descriptor assumptions.
  • Full repository script passes: 4,190 tests, 720 skipped, plus Ruff, ty, codespell, cpplint and MISRA.
  • Upstream carstate replay comparison: 1,011 segments, 0 changed and 0 errors.
  • The existing mutation runner credits queued cases as killed after a worker-pool crash. Independent validation completed all 2,895 cases on each platform: Apple Clang had 2,504 test failures, 89 signals, 3 calibrated timeouts and 299 survivors; GCC had 2,522 test failures, 72 signals, 3 calibrated timeouts and 298 survivors. Neither run had infrastructure errors. Of the native survivors, 97 receive no tests through the existing shared-header routing; four sampled unchanged longitudinal mutations also survive on the unmodified baseline. The other survivors are not all classified. These audits exit 1 against the existing three-entry allowlist; the upstream mutation runner and allowlist are unchanged here.

Related to #2557. I recognize the existing reservation and competing #3702. This implementation is independent; #3702's description also suggested paired-library validation, which is credited in the local validation harness. The comparison covers tested input traces and current descriptor assumptions, not every possible safety behavior.

The separate replay reporting workflow timed out while waiting for the artifact; the replay job completed successfully with the result above. GitHub refused the requested rerun.

@github-actions github-actions Bot added the car safety vehicle-specific safety code label Sep 10, 2026
@joedlai
joedlai marked this pull request as ready for review September 10, 2026 06:05
@joedlai joedlai closed this Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

car safety vehicle-specific safety code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant